ZEPPELIN-501 Notebook search #534

bzz · 2015-12-15T10:54:32Z

What is this PR for?

This PR has for goal to allow the user to search through the code in all the paragraphs and notebook names in all the notebooks

It add a simple 'search bar' to the nav-bar of Zeppelin WebApp, and an in-memory fulltext search index of paragraphs to the backend.

The search is pretty basic now, fine-tuning it for better search over all types of source code will be a subject of further work.

What type of PR is it?

Feature

Todos

Is there a relevant Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-501

How should this be tested?

Outline the steps to test the PR here.

Screenshots (if appropriate)

Questions:

Does the licenses files need update? Yes, added to license files
Is there breaking changes for older versions? No
Does this needs documentation? Yes

This work is a collaboration with @felizbear who contributed major parts of the frontend changes.

vgmartinez · 2015-12-15T10:58:52Z

LGTM...
It is a good feature

corneadoug · 2015-12-15T12:02:02Z

@bzz Could you rewrite the PR description following the PR template (https://github.com/apache/incubator-zeppelin/blob/master/CONTRIBUTING.md#creating-a-pull-request)?

bzz · 2015-12-15T12:43:06Z

@vgmartinez than you for positive feedback!

@corneadoug well, I expected a bit more of the code review, rather than "Wrong formatting of the PR description" ;)

I think I did a good job with JIRA issue. If you never the less feel that something is wrong - please contribute a new description you like better in the form of comment here and I will be willing to updated it.

corneadoug · 2015-12-15T12:53:09Z

@bzz Of course, the code review will come :)

Well, JIRA issue is JIRA issue, PR Template is PR Template :)
You will find that there is a lot of nice categories and questions that will help review the PR.

One good example is:

Does the licenses files need update?

Which answer in this PR's case would be: Yes

But angular-resource was not added to the license file

AhyoungRyu · 2015-12-15T21:27:34Z

docs/rest-api/rest-notebook.md

@bzz I think there are some typos in this sentence! : )

Thank you for proof-reading! Added to the scope below

Conflicts: docs/rest-api/rest-notebook.md

Conflicts: zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java

Conflicts: zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java

`transient` for SerialVersionUID was introduced in apache@68bdb2f#diff-8c6a0b47ceeb47c966f2b058e730b323R26

Conflicts: zeppelin-distribution/src/bin_license/LICENSE

bzz · 2015-12-22T12:10:03Z

@Leemoonsoo All reviews are addressed, I think it's ready to be merged.

Leemoonsoo · 2015-12-22T13:00:53Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/search/SearchService.java

We might not want to print contents of notebook in a log file. Either use LOG.debug or remove 'text' from the log.

Leemoonsoo · 2015-12-22T13:11:17Z

@bzz Great work.
Seems like index is not updated after paragraph remove. Is it possible to be handled?

Leemoonsoo · 2015-12-22T13:19:39Z

Following error is thrown when persisting changes of notebook. The notebook loaded on Zeppelin startup (not created after start up)

java.lang.NullPointerException
    at org.apache.zeppelin.notebook.Note.persist(Note.java:380)
    at org.apache.zeppelin.socket.NotebookServer.updateParagraph(NotebookServer.java:451)
    at org.apache.zeppelin.socket.NotebookServer.onMessage(NotebookServer.java:123)
    at org.apache.zeppelin.socket.NotebookSocket.onMessage(NotebookSocket.java:56)
    at org.eclipse.jetty.websocket.WebSocketConnectionRFC6455$WSFrameHandler.onFrame(WebSocketConnectionRFC6455.java:835)
    at org.eclipse.jetty.websocket.WebSocketParserRFC6455.parseNext(WebSocketParserRFC6455.java:349)
    at org.eclipse.jetty.websocket.WebSocketConnectionRFC6455.handle(WebSocketConnectionRFC6455.java:225)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:667)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:52)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:608)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:543)
    at java.lang.Thread.run(Thread.java:745)

bzz · 2015-12-23T02:06:45Z

@Leemoonsoo thank you for thought testing and review! My bad, scope is updated now, will address all the feedback, and let you know.

…uctor

bzz · 2015-12-23T08:22:54Z

@Leemoonsoo all the feedback has been addressed now

…sed impl

Leemoonsoo · 2015-12-23T10:22:42Z

Tested and works really well. Thanks for really useful new feature.
LGTM!

bzz · 2015-12-23T10:26:37Z

Thank you! Merging if there are no other discussions

bzz and others added 24 commits December 15, 2015 18:58

Search: form added to navbar in frontend

c5928f9

Search: backend REST API scetch /notebooks/search?q=... added

7880237

Search: disabling UI on disconnect

3900b60

Get rid of compiler warnings

bc2458a

Search: add search/result-list and switch to

163a465

Search: backend indexing using Lucene added to zengine

c2c2a52

Search: highlighting added using fragments

db246fa

style search widget for notebook search

11127f0

Search: merge first 3 fragments + full paragraph in search result

9ca8628

highlight syntax and search terms in search results

865925c

update search results view: add panels for results

227c6b4

center search results on screen

aa5ddb3

Search: nuke compiler warnings in NotebookRestApi

6ccd6f1

Search: make checkstyle happy

7aad5cf

Search: make jshint happy

e363ed4

Refatoring - removing old annotations

5c1e3e4

Search: refactoring, rename fragment -> snippet

98f4e59

Search: adding search API docs

63a4e05

Search: adding tests

08fe806

Search: refatoring, move SearchService from NotebookRepoSync -> Notebook

b2b93c4

Search: re-index note on every change

09d44d2

redirect to notebook and scroll to paragraph from search view

1e1357c

clear search field on navigation to home view (/)

71ec51f

ZEPPELIN-501: reverting accidental changes

82c7dd7

AhyoungRyu reviewed Dec 15, 2015
View reviewed changes

bzz added 14 commits December 18, 2015 15:31

ZEPPELIN-501: refactoring + handling index Create\Delete

825b266

ZEPPELIN-501: fixing NPE in tests \w mocks

8c0f29a

Merge branch 'master' into notebook-search

6da1dc9

ZEPPELIN-501: add missing logger

0ce8a92

Merge branch 'master' into notebook-search

eb7878a

Conflicts: docs/rest-api/rest-notebook.md

ZEPPELIN-501: refactoring, cleaning compiler warnings in Note.java

44235eb

Merge branch 'master' into notebook-search

2b2f8dc

Conflicts: zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java

Merge branch 'master' into notebook-search

6a3906f

Conflicts: zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java

ZEPPELIN-501: refactoring, cleaning compiler warnings in Notebook.java

40cf9e8

ZEPPELIN-501: refactoring, compiler warning in Paragraph.java

3f20904

`transient` for SerialVersionUID was introduced in apache@68bdb2f#diff-8c6a0b47ceeb47c966f2b058e730b323R26

ZEPPELIN-501: update notebooks on save + tests

e915a69

ZEPPELIN-501: fix bug (merge artefact) that failed CI befor

5f47890

ZEPPELIN-501: validate notes before update index

da69c07

Merge branch 'master' into notebook-search

f30e6c8

Conflicts: zeppelin-distribution/src/bin_license/LICENSE

Leemoonsoo reviewed Dec 22, 2015
View reviewed changes

bzz added 6 commits December 23, 2015 11:10

ZEPPELIN-501: stop printing whole note in logs

d2b44ac

ZEPPELIN-501: fix NPE on double-delete

cf44b1f

ZEPPELIN-501: fix NPE on Note creation by NotebookRepo, \wo DI constr…

73a28bb

…uctor

ZEPPELIN-501: update index on paragraph delete

78f69a3

ZEPPELIN-501: fix + test for bug in index key structure

c77d53b

ZEPPELIN-501: muting more search logs info->debug

e3af25c

ZEPPELIN-501: refactoring, extract SearchService interface + Lucen-ba…

c7f1d35

…sed impl

asfgit closed this in 82de508 Dec 23, 2015

ZEPPELIN-501 Notebook search #534

ZEPPELIN-501 Notebook search #534

Uh oh!

Conversation

bzz commented Dec 15, 2015

What is this PR for?

What type of PR is it?

Todos

Is there a relevant Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

vgmartinez commented Dec 15, 2015

Uh oh!

corneadoug commented Dec 15, 2015

Uh oh!

bzz commented Dec 15, 2015

Uh oh!

corneadoug commented Dec 15, 2015

Uh oh!

AhyoungRyu Dec 15, 2015

Choose a reason for hiding this comment

Uh oh!

bzz Dec 16, 2015

Choose a reason for hiding this comment

Uh oh!

bzz commented Dec 22, 2015

Uh oh!

Leemoonsoo Dec 22, 2015

Choose a reason for hiding this comment

Uh oh!

Leemoonsoo commented Dec 22, 2015

Uh oh!

Leemoonsoo commented Dec 22, 2015

Uh oh!

bzz commented Dec 23, 2015

Uh oh!

bzz commented Dec 23, 2015

Uh oh!

Leemoonsoo commented Dec 23, 2015

Uh oh!

bzz commented Dec 23, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants